-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Removing loop name from asyncio.create_task #489
Removing loop name from asyncio.create_task #489
Conversation
LGTM |
ydb/_topic_common/common.py
Outdated
def wrap_create_asyncio_task(func: typing.Callable, task_name: str, *args, **kwargs): | ||
if sys.hexversion < 0x03080000: | ||
return asyncio.create_task(func(*args, **kwargs)) | ||
return asyncio.create_task(func(*args, **kwargs), name=task_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems weird how we pass name and args. This wrapper could get already created task and use task.set_name(name)
in case of python 3.8+. In this case you will don't have any problems between asyncio.create_task()
and loop.create_task()
because they both return a Task you can pass into your wrapper
Also this wrapper could have a better name, because the current one doesn't explain anything we do with the task.
I don't have anything against hexversion, but I'm sure we can omit this check on every wrapper call. We could use this split during function declaration stage. In this case, if we will have only one if
call, think about switching to version_info
for better readability (i'm not insist here)
Adding python 3.7 support for the ydb topic.
Pull request type
Please check the type of change your PR introduces:
Other information
Removing loop name from asyncio.create_task, since it only appeared in python3.8.